Skip to content

feat(hooks): add run_windows support#10202

Merged
jdx merged 4 commits into
jdx:mainfrom
risu729:codex-20260602-012911-022346
Jun 6, 2026
Merged

feat(hooks): add run_windows support#10202
jdx merged 4 commits into
jdx:mainfrom
risu729:codex-20260602-012911-022346

Conversation

@risu729

@risu729 risu729 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add run_windows to inline hook tables, matching task-style Windows command selection
  • render templates for both hook command variants and skip Windows-only hooks on non-Windows platforms
  • update hook docs/schema and add e2e coverage

Addresses #10139.

Testing

  • RUSTC_WRAPPER= cargo test hooks::tests
  • MISE_TRUSTED_CONFIG_PATHS="$PWD" RUSTC_WRAPPER= mise run build
  • MISE_TRUSTED_CONFIG_PATHS="$PWD" CARGO_TARGET_DIR=/home/risu/.cache/cargo-target/mise-409845c58888dcc6 RUSTC_WRAPPER= mise run --skip-deps test:e2e e2e/config/test_hooks_run_windows
  • git diff --check

Summary by CodeRabbit

  • New Features

    • Added support for Windows-specific hook commands that run only on Windows.
  • Documentation

    • Updated hook docs with examples for Windows-only hooks and clarified validation messaging (both command fields must be strings; array syntax is unsupported).
  • Tests

    • Added end-to-end tests verifying platform gating and template handling for Windows-specific hooks.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ed4bf9a7-03c1-44bf-9329-6dbd5891c945

📥 Commits

Reviewing files that changed from the base of the PR and between 1ad5e38 and b924a87.

📒 Files selected for processing (2)
  • docs/hooks.md
  • src/hooks.rs
💤 Files with no reviewable changes (1)
  • docs/hooks.md

📝 Walkthrough

Walkthrough

Adds optional run_windows to inline hook tables, updates schema and parsing to accept either run or run_windows, renders and executes only the platform-active command (Windows prefers run_windows), and includes unit, E2E tests and docs updates.

Changes

Windows-specific hook command support

Layer / File(s) Summary
Schema and type contracts
schema/mise.json, src/hooks.rs
JSON schema accepts run_windows and requires either run or run_windows; HookRunTable added; HookDef::Run and HookAction::Run now use optional run/run_windows fields.
Rendering and execution selection
src/hooks.rs
Script hooks adapted to the new optional fields; template rendering selects and renders only the active command (Windows prefers run_windows); execution picks the active command and returns early when none is applicable.
Tests (unit + E2E)
src/hooks.rs, e2e/config/test_hooks_run_windows
Unit test verifies TOML parsing into HookAction::Run; E2E test asserts Unix-hook output appears and Windows-only hook output is absent on Unix.
Documentation updates
docs/hooks.md
Docs explain run_windows behavior, provide a postinstall example using both fields, and clarify validation messaging about string types and disallowed array syntax for run.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hops cross platforms wide,
With run_windows as our guide,
Now hooks choose which path to run,
Windows or Unix, each gets one,
Templates sleep where they don't apply. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(hooks): add run_windows support' directly and accurately summarizes the main feature addition. It is concise, specific, and clearly indicates the primary change in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds run_windows support to inline hook tables, matching the existing task-style Windows command selection. Both render_templates and execute use cfg!(windows) compile-time gating so run_windows content is never evaluated on non-Windows platforms, and a custom Deserialize impl validates that at least one of run or run_windows is always present.

  • HookRunTable replaces the former flat Run { run, shell } variant and carries the new run_windows field; the manual deserializer rejects tables where both fields are absent.
  • render_templates and execute consistently select run_windows (preferred) or run at compile time via cfg!(windows), so templates in run_windows are skipped entirely on non-Windows.
  • The JSON schema switches from required: ["run"] to an anyOf requiring either run or run_windows, and an e2e test confirms the platform-gating behavior including that exec() templates in run_windows do not fire on Linux/macOS.

Confidence Score: 5/5

Change is safe to merge; platform selection is consistent across parsing and execution, and existing hook behavior on all platforms is unaffected.

The template rendering and execution paths both use the same cfg!(windows) guard, so run_windows content is never touched on non-Windows. The custom deserializer enforces the at-least-one invariant at parse time, the schema mirrors that constraint, and the e2e test explicitly exercises all three cases (unix-only run, windows-only run, and the mixed case with a Tera exec() guard).

No files require special attention.

Important Files Changed

Filename Overview
src/hooks.rs Adds HookRunTable struct with a custom deserializer enforcing at-least-one-of run/run_windows; render_templates and execute both use cfg!(windows) so run_windows templates are never evaluated on non-Windows
schema/mise.json Adds run_windows field and replaces required: ["run"] with anyOf requiring either run or run_windows, correctly mirroring the Rust validation
e2e/config/test_hooks_run_windows E2E test verifies run_windows-only hooks are skipped on non-Windows, platform-mixed hooks run the run variant, and run_windows templates (including exec()) are not evaluated on non-Windows
docs/hooks.md Documents the new run_windows field with a TOML example and updates the constraint note to mention both run and run_windows must be strings

Reviews (4): Last reviewed commit: "refactor(hooks): inline run_windows sele..." | Re-trigger Greptile

Comment thread src/hooks.rs Outdated
@risu729

risu729 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Greptile’s template-rendering concern in f7396e2 by rendering only the selected hook command variant, so inactive run_windows templates are not evaluated on non-Windows platforms.

This comment was generated by an AI coding assistant.

@risu729

risu729 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

This comment was generated by an AI coding assistant.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the PR details and changed files
gh pr view 10202 --repo jdx/mise --json title,body,files,commits,headRefName

echo "--- DIFF ---"
gh pr diff 10202 --repo jdx/mise

Length of output: 12953


🏁 Script executed:

#!/bin/bash
# Check HookDef serde attributes around the Run variant and any related derive usage
rg -n "HookDef|serde\|deserialize\|Deserialize" src/hooks.rs | head -80

echo "--- HookDef enum definition ---"
ast-grep --pattern 'pub enum HookDef {
  $$$
}' --lang rust

echo "--- check for any derive on HookDef ---"
rg -n -B2 "enum HookDef" src/hooks.rs

Length of output: 2063



@risu729 Nice work — the implementation is clean and well-tested. Here's my full review:


Review

src/hooks.rs

Custom deserializer for HookRunTable — The manual Deserialize impl with the Helper pattern correctly enforces the "at least one of run/run_windows" invariant at parse time and aligns with the anyOf schema constraint. One thing to be aware of: because HookDef uses #[serde(untagged)], the custom error message "hook run table must define 'run' or 'run_windows'" is swallowed by serde's untagged fallthrough logic, so a user who writes { shell = "bash -c" } (no run/run_windows) will see a generic "data did not match any variant of untagged enum HookDef" error rather than the helpful custom message. This is a pre-existing design trade-off of the untagged enum, not something introduced here — just worth noting for the future if ergonomic error messages become a goal.

deny_unknown_fields on the inner Helper struct — This is actually a minor improvement over the old Run { run: String, shell: Option<String> } named-field variant (which had no such guard). Typos like { runn = "..." } now fall through to a clear mismatch rather than being silently ignored. ✅

select_run / select_run_mut — The Windows-fallback logic (run_windows.or(run) on Windows, run only on non-Windows) matches the documented task-style semantics exactly. The template-rendering gate (if let Some(s) = select_run_mut(...)) ensures inactive variant templates are never evaluated — the fix from f7396e2 is correct. ✅

execute() early returnselect_run returning None (windows-only hook on non-Windows) causes a clean return Ok(()). ✅

Backward compatibilityRunString and script_hook_action both hardcode run_windows: None, preserving existing behavior for all hook types that don't support the new field. ✅

e2e/config/test_hooks_run_windows

Covers both cases on non-Windows cleanly:

  • { run_windows = ... } alone → skipped ✅
  • { run = ..., run_windows = ... }run selected ✅

A Windows-side e2e test would be the natural complement, but that's inherently gated by Windows CI availability.

schema/mise.json

The anyOf with required: ["run"] / required: ["run_windows"] correctly mirrors the runtime validation and permits run_windows-only tables. ✅

docs/hooks.md

Clear and self-contained. The example TOML with both unix_default_inline_shell_args and windows_default_inline_shell_args is a good teaching aid. ✅


Overall: The PR is solid. No blocking issues. The serde error-message caveat is purely informational — it won't affect correctness.

This comment was generated by an AI coding assistant.

[approve]


🧠 Learnings used
Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:12.203Z
Learning: When posting comments on GitHub PRs or discussions, include a note that the comment was AI-generated (e.g., '*This comment was generated by an AI coding assistant.*').

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:18.847Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: Applies to **/*_windows.rs : Windows-specific tests are in e2e-win/ directory and Windows-specific implementations are in files ending with _windows.rs

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Build and test using mise run commands (build, test, test:unit, test:e2e, snapshots, lint, lint-fix, format) rather than running cargo or other tools directly

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Run `hk install --mise` once to set up pre-commit hooks. Run `mise run lint-fix` before committing. Use `mise run test:e2e [test_filename]...` for running specific e2e tests, never execute test files directly.

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2026-05-31T15:11:40.047Z
Learning: Applies to **/*e2e*.{rs,test.rs} : Run end-to-end tests using `mise run test:e2e [test_filename]...` instead of executing them directly

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2026-06-03T23:51:16.391Z
Learning: Applies to **/tests/**/*.rs : Run e2e tests using `mise run test:e2e [test_filename]...` instead of executing them directly

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2026-05-31T15:11:40.047Z
Learning: Applies to **/*.{test.rs,unit.rs} : Run unit tests using `mise run test:unit`

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-06-04T13:45:55.155Z
Learning: Execute e2e tests using `mise run test:e2e [test_filename]...`

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-06-03T23:51:16.391Z
Learning: Execute e2e tests using `mise run test:e2e [test_filename]...`

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-05-31T15:11:54.818Z
Learning: Execute e2e tests via `mise run test:e2e [test_filename]...`

@risu729

risu729 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

This comment was generated by an AI coding assistant.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@risu729 I'll kick off a fresh review of the latest changes.

This comment was generated by an AI coding assistant.


🧠 Learnings used
Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:12.203Z
Learning: When posting comments on GitHub PRs or discussions, include a note that the comment was AI-generated (e.g., '*This comment was generated by an AI coding assistant.*').

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: Applies to **/*_windows.rs : Windows-specific tests are in e2e-win/ directory and Windows-specific implementations are in files ending with _windows.rs

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:18.847Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Build and test using mise run commands (build, test, test:unit, test:e2e, snapshots, lint, lint-fix, format) rather than running cargo or other tools directly

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-03T17:35:47.414Z
Learning: Run `hk install --mise` once to set up pre-commit hooks. Run `mise run lint-fix` before committing. Use `mise run test:e2e [test_filename]...` for running specific e2e tests, never execute test files directly.

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2026-05-31T15:11:40.047Z
Learning: Applies to **/*e2e*.{rs,test.rs} : Run end-to-end tests using `mise run test:e2e [test_filename]...` instead of executing them directly

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2026-06-03T23:51:16.391Z
Learning: Applies to **/tests/**/*.rs : Run e2e tests using `mise run test:e2e [test_filename]...` instead of executing them directly

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/development.mdc:0-0
Timestamp: 2026-05-31T15:11:40.047Z
Learning: Applies to **/*.{test.rs,unit.rs} : Run unit tests using `mise run test:unit`

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-06-04T13:45:55.155Z
Learning: Execute e2e tests using `mise run test:e2e [test_filename]...`

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-06-03T23:51:16.391Z
Learning: Execute e2e tests using `mise run test:e2e [test_filename]...`

Learnt from: CR
Repo: jdx/mise PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2026-05-31T15:11:54.818Z
Learning: Execute e2e tests via `mise run test:e2e [test_filename]...`
✅ Action performed

Full review finished.

@risu729

risu729 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

This comment was generated by an AI coding assistant.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the latest PR details and commits
gh pr view 10202 --repo jdx/mise --json title,state,headRefName,commits,updatedAt

echo "--- RECENT COMMITS ---"
gh pr view 10202 --repo jdx/mise --json commits --jq '.commits[-5:] | .[] | {oid: .oid, message: .messageHeadline, date: .committedDate}'

echo "--- FULL DIFF ---"
gh pr diff 10202 --repo jdx/mise

Length of output: 11794



@risu729 The latest commit (refactor(hooks): inline run_windows selection, b924a87) is a clean improvement over the previous approach. Here's a focused review of the new changes:

refactor(hooks): inline run_windows selection

Template rendering (render_templates) — The inlined selection logic is correct. Gating shell and ignored_shell rendering inside the if let Some(s) = run block is sound: on non-Windows, a run_windows-only hook produces None from run.as_mut(), so no templates are evaluated at all before the early return in execute. ✅

execute function — The inlined run_windows.as_deref().or(run.as_deref()) / run.as_deref() pattern and let Some(run) = run else { return Ok(()) } guard are clear and correct. Removing the select_run/select_run_mut helper methods makes the platform-selection logic directly visible at the point of use — a readability win. ✅

Unit testrun_table_supports_run_windows verifies that all three fields (run, run_windows, shell) round-trip through TOML deserialization correctly. The earlier test commit covers inactive-template and platform-selection behavior separately. ✅


Overall: No blocking issues. The refactor tightens the implementation without changing semantics. The PR remains in good shape.

This comment was generated by an AI coding assistant.

[approve]


🧠 Learnings used
Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:12.203Z
Learning: When posting comments on GitHub PRs or discussions, include a note that the comment was AI-generated (e.g., '*This comment was generated by an AI coding assistant.*').

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:18.847Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: When posting comments on GitHub PRs or discussions, always include a note that the comment was AI-generated

Learnt from: CR
Repo: jdx/mise PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-31T15:12:28.703Z
Learning: Applies to **/*_windows.rs : Windows-specific tests are in e2e-win/ directory and Windows-specific implementations are in files ending with _windows.rs

@risu729 risu729 marked this pull request as ready for review June 5, 2026 18:32
@jdx jdx merged commit c313cde into jdx:main Jun 6, 2026
34 checks passed
@risu729 risu729 deleted the codex-20260602-012911-022346 branch June 6, 2026 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants